-
Couldn't load subscription status.
- Fork 2k
Lazy mcp client and toolcallback resolution #4671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Lazy mcp client and toolcallback resolution #4671
Conversation
Resolves critical timing issues in MCP client initialization and
tool callback resolution that prevented proper registration of
MCP-annotated beans.
- MCP clients created after all singleton beans initialized
- Implement SmartInitializingSingleton for deferred client creation
via McpSyncClientInitializer and McpAsyncClientInitializer
- Tool callbacks resolved at execution instead configuration
- Store ToolCallbackProvider instances in ChatClient and resolve
lazily at execution time (call/stream)
- Filter MCP providers from StaticToolCallbackResolver
- Inconsistent list reference handling
- Add mcpClientsReference() methods for proper reference sharing
Breaking Changes:
- MCP providers no longer in static resolver (transparent)
- Client creation timing changed (transparent)
Fixes: spring-projects#4670, spring-projects#4618
Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
98ad131 to
2b8bd13
Compare
| .toolContextToMcpMetaConverter( | ||
| toolContextToMcpMetaConverter.getIfUnique(() -> ToolContextToMcpMetaConverter.defaultConverter())) | ||
| .mcpClients(mcpClients) | ||
| .mcpClientsReference(mcpClients) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it can use clients instead of mcpClients. after all. the mcpClients is deprecated and mcpClientsReference is too long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ilayaperumalg The lazy resolution improvements look solid overall.
• The mutate() path needs to retain any pending ToolCallbackProvider instances so they persist after mutation.
• The annotation-scanner condition change currently reverses user intent when disabling the scanner; enabled=false should keep it off rather than re-enable it.
| @Deprecated(since = "1.1.0", forRemoval = true) | ||
| @AutoConfiguration(after = McpClientAnnotationScannerAutoConfiguration.class) | ||
| @ConditionalOnClass(McpLogging.class) | ||
| @ConditionalOnProperty(prefix = McpClientAnnotationScannerProperties.CONFIG_PREFIX, name = "enabled", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @tzolov – flipping the condition to havingValue="false" unintentionally re-enables this (deprecated) auto-configuration whenever a user sets spring.ai.mcp.client.annotation-scanner.enabled=false. In 1.1.0‑M2 that property meant “turn it off”; with this change the same setting now activates it, so existing apps that disabled the scanner will regress. If the goal is to keep it off by default, the condition should probably remain tied to true (with a new default of false) or be removed entirely.
| * settings are replicated from this {@link ChatClientRequest}. | ||
| */ | ||
| @Override | ||
| public Builder mutate() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @tzolov – DefaultChatClientRequestSpec.mutate() only copies the already-resolved toolCallbacks list into the new builder and drops the still‑pending toolCallbackProviders. If a request spec is configured with toolCallbacks(provider) and then mutated before executing, the new client silently loses the provider and no MCP tools are ever resolved. (Repro: call chatClient.prompt().toolCallbacks(provider).mutate().build().prompt().call() – the provider is never invoked.) Please carry the providers over as well, e.g. add defaultToolCallbacks(this.toolCallbackProviders.toArray(new ToolCallbackProvider[0])) before returning the builder, or otherwise ensure they survive mutation.
Fixes critical timing issues where MCP clients were created before all annotated beans were scanned, and tool callbacks were resolved too early during ChatClient configuration.
Problems Fixed
Solution
McpSyncClientInitializerandMcpAsyncClientInitializerclasses defer client creation until after all singletons are instantiatedcall()/stream())StaticToolCallbackResolverto enable lazy resolutionmcpClientsReference()methods to properly share list referencesKey Changes
Configuration:
McpClientAutoConfiguration.java- Added initializer classesMcpToolCallbackAutoConfiguration.java- Uses reference sharingMcpClientSpecificationFactoryAutoConfiguration.java- Deprecated (disabled by default)Tool Resolution:
DefaultChatClient.java- Lazy provider resolutionToolCallingAutoConfiguration.java- MCP provider filteringProviders:
mcpClientsReference()toSyncMcpToolCallbackProviderandAsyncMcpToolCallbackProviderBreaking Changes
Deprecated (still functional):
mcpClients(List)andmcpClients(varargs)→ UsemcpClientsReference(List)insteadBehavioral (transparent to users):
Migration
Most users: No changes required - fixes are transparent and backward compatible.
Custom MCP providers: Update to use
mcpClientsReference()instead of deprecatedmcpClients()methods.Resolves: #4670, #4618